Skip to content

systemtests: add configurable timeout per test case#735

Open
AdityaGupta716 wants to merge 1 commit intoprecice:developfrom
AdityaGupta716:systemtests/add-timeout-support
Open

systemtests: add configurable timeout per test case#735
AdityaGupta716 wants to merge 1 commit intoprecice:developfrom
AdityaGupta716:systemtests/add-timeout-support

Conversation

@AdityaGupta716
Copy link
Copy Markdown

Adds an optional timeout field to entries in tests.yaml, allowing per-test timeout configuration instead of a single hardcoded global value.

Closes #371

Changes

  • metadata_parser/metdata.py — added timeout: int = 600 to ReferenceResult
  • systemtests/TestSuite.py — parses optional timeout from tests.yaml; accepts both integer (600) and string (600s) formats; validates that the value is positive
  • systemtests/Systemtest.py — renamed GLOBAL_TIMEOUT to BUILD_TIMEOUT (docker build stays fixed); solver run and fieldcompare now use the per-test configured timeout
  • tools/tests/README.md — documented the new optional field with an example

Example usage in tests.yaml

- path: heat-exchanger
  case_combination:
    - fluid-openfoam
    - solid-calculix
  reference_result: ./heat-exchanger/reference-results/...
  timeout: 3600

Backward compatible — existing entries without timeout use the previous default of 600s.

Checklist

  • I added a summary of any user-facing changes in changelog-entries/371.md
  • I will remember to squash-and-merge

@AdityaGupta716 AdityaGupta716 force-pushed the systemtests/add-timeout-support branch from 51e92a0 to 1ffd4f0 Compare March 2, 2026 10:23
@AdityaGupta716
Copy link
Copy Markdown
Author

Hi @MakisH, the timeout is currently attached to ReferenceResult since that's where per-test configuration lives. Happy to move it to a dedicated field or separate dataclass if you prefer a different structure. Also kept BUILD_TIMEOUT fixed since build time feels infrastructure-dependent rather than simulation-dependent — let me know if you'd like that configurable too.

@MakisH MakisH added GSoC Contributed in the context of the Google Summer of Code systemtests labels Mar 6, 2026
@precice-bot
Copy link
Copy Markdown
Collaborator

This pull request has been mentioned on preCICE Forum on Discourse. There might be relevant details there:

https://precice.discourse.group/t/gsoc-2026-aditya-gupta/2773/4

Copy link
Copy Markdown
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! This is going in a good direction, but needs some further refactoring and cleanup. See some comments.

class ReferenceResult:
path: Path
case_combination: CaseCombination

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed it



GLOBAL_TIMEOUT = 900
BUILD_TIMEOUT = 900
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why rename it?
How does this fit with the changes suggested/discussed in #731?

Let's discuss directly in the Changelog entry thread.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer renaming, GLOBAL_TIMEOUT stays as is, now overridable via PRECICE_SYSTEMTEST_TIMEOUT environment variable as suggested .

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated changes, see #731 (comment) and revert.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed it

timeout: 3600
```
The optional `timeout` field (in seconds, default: 600) sets the maximum time allowed for the solver run and fieldcompare phases. Use this for long-running tutorials such as `heat-exchanger` or `turek-hron-fsi3`.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be able to test this PR, you could set a timeout to one tutorial. We can remove it again before merging. For example, the Nutils cases are much slower to run than the rest, while the new DuMux cases take the longest to build.

Note that the default recently changed to 900 in #696

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added timeout: 1200 to nutils_test. Default now references to GLOBAL_TIMEOUT

@@ -0,0 +1 @@
- Add optional `timeout` field to `tests.yaml` entries, allowing per-test timeout configuration. Defaults to 600s for backward compatibility. Applies to the solver run and fieldcompare phases.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The total timeout now per case would be the build timeout + the case-specific timeout = 900s + 600s = 1500s = 25min by default. Is that intended? That would be much larger than what we currently have.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the default mention from changelog. The per-test timeout only applies to solver run and fieldcompare because build time depends on infrastructure , not on the simulation itself. Default behavior is unchanged all three phases default to GLOBAL_TIMEOUT . The per-test timeout only takes effect when set in tests.yaml.

name: str
cases_of_tutorial: Dict[Tutorial, List[CaseCombination]]
reference_results: Dict[Tutorial, List[ReferenceResult]]
timeouts: Dict[Tutorial, List[int]] = field(default_factory=dict)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a (dictionary with a) list per Tutorial and not a single variable per CaseCombination?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i followed it from cases_of_tutorial and reference_result thought should do the same here should i change it ?

arguments: SystemtestArguments
case_combination: CaseCombination
reference_result: ReferenceResult
timeout: int = 600
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This number seems to now be hard-coded in different places.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed it , now timeout: int = GLOBAL_TIMEOUT

for tutorial in tutorials:
for case, reference_result in zip(
test_suite.cases_of_tutorial[tutorial], test_suite.reference_results[tutorial]):
timeouts = test_suite.timeouts.get(tutorial, [600] * len(test_suite.cases_of_tutorial[tutorial]))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why hard-coding the 600 here? Same for the generate_reference_results.py.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed it

@AdityaGupta716 AdityaGupta716 force-pushed the systemtests/add-timeout-support branch from e4b349c to d7345a4 Compare March 26, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

GSoC Contributed in the context of the Google Summer of Code systemtests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Timeout to kill the simulation when it "hangs"

3 participants